Drop _emerge/getloadavg.py #1383
Conversation
|
I searched a bit and found comment related to #589:
As I recall, the getloadavg function was added in 8607a39 due to the function being missing for python built with uclibc back then. I've found that this is still an issue with current autobuilds found in http://ftp.romnet.org/gentoo/releases/amd64/autobuilds/current-stage3-amd64-uclibc-vanilla/, for example this is with stage3-amd64-uclibc-vanilla-20200619.tar.bz2:
```
Python 3.7.7 (default, May 31 2020, 21:22:59)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.getloadavg
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: module 'os' has no attribute 'getloadavg'
```
We should be able to remove the /proc/loadaverage usage if all of your supported libc implementations have
getloadaverage(). It was lacking in uclibc at the time of 8607a39.
Note that `os.getloadavg()` is already preferred when it is detected in `lib/_emerge/getloadavg.py` here:
```python
from portage import os
getloadavg = getattr(os, "getloadavg", None)
if getloadavg is None:
def getloadavg():
```
It would be basically harmless to keep this getloadavg fallback implementation, in case we ever encounter python with `os.getloadavg()` missing again.
It could be helpful for people running prefix portage with exotic kernel / libc / python implementations. We could remove it and then add it back again if anyone ever finds that they need it, but what's the motivation to remove it given that it's basically harmless whenever a true `os.getloadavg()` is available?
If we remove it from the master branch, then the prefix branch might want to keep it in order to preserve maximum flexibility here. @grobian, @heroxbd, @chewi, and @thesamesam how do you guys feel about this?
|
|
Hi @zmedico os.getloadavg() returns data everywhere I tried. Should it be unavailable, I think we should just ignore it, and disable or bark or something when load average limits are used. It's not like loadaverage means the same on every platform anyway. As for keeping it in Prefix: I'd say not. /proc is only available on Linux this way AFAICT, so it's hardly useful on other platforms. |
|
Sounds good to me, although I don't do much with alternative libcs. |
Flowdalic
force-pushed
the
drop-getloadavg.py
branch
from
6893ab4 to
f82b663
Compare
|
On reflection, please update the commit message to..
|
Portage's custom getloadavg.py wrapper was added with 8607a39 ("If necessary, use /proc/loadavg to emulate os.getloadavg()."). All platforms we care about provide os.getloadavg() now, so this can be dropped. Zac did a nice analysis of the historical background in gentoo#1383 (comment) to summarize: python on uclibc did not had os.getloadavg() in the past, but uclibc (and uclibc-ng) where retired in gentoo [1]. 1: https://www.gentoo.org/support/news-items/2021-08-18-uclibc-ng-retirement.html Signed-off-by: Florian Schmaus <[email protected]>
Flowdalic
force-pushed
the
drop-getloadavg.py
branch
from
f82b663 to
f22ae56
Compare
|
I've updated the commit message
this behavior would change with this PR applied. In the past, we simply ignored a missing So we where ignoring "getloadavg() failures" in the past, but after this PR, we would fail. |
|
I'd kind of prefer it if we caught it not existing at all (not just OSError) but if others are OK with failing hard, I guess it's OK with me. |
I have no strong opinion. I am just always happy if old cruft is remove from a codebase. It seems like we do not need this any more, especially with uclibc gone, but I guess the only way to find out for sure is to give it a shot. |
|
The issue is more with ports to new platforms IMO. |
CC @zmedico You added the
getloadavg()wrapper to portage with 8607a39dc9d9. However, the commit message lacks a rationale why such a wrapper became necessary. This PR assumes that the reasons for the wrapper are now no longer existent. Could you please clarify if this is true? Otherwise we should consider adding a code comment to_emerge/getloadavg.pyexplaining the reason for the wrapper.